feat(add-comment): support reply_to_id for discussion threading from any trigger#24367
feat(add-comment): support reply_to_id for discussion threading from any trigger#24367
Conversation
When an agent outputs a reply_to_id field in the add_comment message, the handler now uses it to post a threaded comment to a Discussion. This works for both: - discussion/discussion_comment events with explicit item_number - workflow_dispatch events where item_number refers to a discussion (via the existing 404-fallback path to the GraphQL discussion API) resolveTopLevelDiscussionCommentId is applied to reply_to_id to handle the case where the caller passes a nested reply node ID rather than a top-level comment node ID." Agent-Logs-Url: https://github.com/github/gh-aw/sessions/53f201fe-1abf-4a8a-9541-6de36a26c892 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot make sure the aw_context message contains the reply_to_id information |
Add comment_node_id (GraphQL node ID) to the aw_context object built by buildAwContext() and resolveItemContext(). For discussion and discussion_comment events, this captures payload.comment.node_id so that dispatched specialist workflows receive the GraphQL node ID of the triggering discussion comment via aw_context.comment_node_id. The specialist can pass this value as reply_to_id in an add_comment output to thread its reply under the triggering comment, completing the CentralRepoOps dispatch chain end-to-end. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2ac8e162-7a35-4423-a934-4c4d4c91c3c2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2ac8e162-7a35-4423-a934-4c4d4c91c3c2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit c4ef606. The flow is:
|
There was a problem hiding this comment.
Pull request overview
This PR extends the add-comment safe output handler to support explicit discussion reply threading (reply_to_id) from non-discussion_comment triggers (notably workflow_dispatch), and propagates the triggering discussion comment’s GraphQL node ID through aw_context so dispatched workflows can reply in-thread.
Changes:
- Update
add_commentto honormessage.reply_to_idfor Discussions (including the REST→404→GraphQL fallback path) and resolve nested replies to top-level parents. - Extend
aw_contextto includecomment_node_id(payload comment GraphQL node ID) for discussion/discussion_comment events. - Add/adjust unit tests to cover
reply_to_idbehavior andcomment_node_idcontext propagation.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/add_comment.cjs | Uses reply_to_id to thread discussion replies outside the auto-threaded discussion_comment path; resolves nested reply IDs to top-level. |
| actions/setup/js/add_comment.test.cjs | Adds tests for reply_to_id threading, top-level resolution, and precedence rules. |
| actions/setup/js/aw_context.cjs | Adds comment_node_id to resolveItemContext() and buildAwContext() for discussion threading support in dispatched workflows. |
| actions/setup/js/aw_context.test.cjs | Updates expectations and adds coverage for comment_node_id extraction. |
| actions/setup/js/generate_aw_info.test.cjs | Adds coverage to ensure aw_context with comment_node_id is accepted and persisted. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 1
actions/setup/js/add_comment.cjs
Outdated
| let replyToId; | ||
| if (context.eventName === "discussion_comment" && !hasExplicitItemNumber) { | ||
| // When triggered by a discussion_comment event, thread the reply under the triggering comment. | ||
| replyToId = await resolveTopLevelDiscussionCommentId(githubClient, context.payload?.comment?.node_id); | ||
| } else if (message.reply_to_id) { | ||
| // Allow the agent to explicitly specify a reply_to_id (e.g. for workflow_dispatch-triggered | ||
| // workflows that know the target comment node ID). Apply resolveTopLevelDiscussionCommentId | ||
| // to handle cases where the caller passes a reply node ID instead of a top-level one. | ||
| replyToId = await resolveTopLevelDiscussionCommentId(githubClient, message.reply_to_id); |
There was a problem hiding this comment.
message.reply_to_id is passed directly into resolveTopLevelDiscussionCommentId() / GraphQL without type/format normalization. If an agent emits a non-string (number/object) or whitespace-only value, the subsequent node(id: $nodeId) query can fail and cause the handler to error. Consider coercing to String(...), trimming, and ignoring/ warning when the result is empty; ideally centralize this normalization so the same validated value is used for both the main discussion path and the 404 discussion fallback path.
| let replyToId; | |
| if (context.eventName === "discussion_comment" && !hasExplicitItemNumber) { | |
| // When triggered by a discussion_comment event, thread the reply under the triggering comment. | |
| replyToId = await resolveTopLevelDiscussionCommentId(githubClient, context.payload?.comment?.node_id); | |
| } else if (message.reply_to_id) { | |
| // Allow the agent to explicitly specify a reply_to_id (e.g. for workflow_dispatch-triggered | |
| // workflows that know the target comment node ID). Apply resolveTopLevelDiscussionCommentId | |
| // to handle cases where the caller passes a reply node ID instead of a top-level one. | |
| replyToId = await resolveTopLevelDiscussionCommentId(githubClient, message.reply_to_id); | |
| const normalizedExplicitReplyToId = message.reply_to_id === undefined || message.reply_to_id === null | |
| ? null | |
| : String(message.reply_to_id).trim(); | |
| if (message.reply_to_id !== undefined && message.reply_to_id !== null && !normalizedExplicitReplyToId) { | |
| core.warning("Ignoring empty discussion reply_to_id after normalization"); | |
| } | |
| let replyToId; | |
| if (context.eventName === "discussion_comment" && !hasExplicitItemNumber) { | |
| // When triggered by a discussion_comment event, thread the reply under the triggering comment. | |
| replyToId = await resolveTopLevelDiscussionCommentId(githubClient, context.payload?.comment?.node_id); | |
| } else if (normalizedExplicitReplyToId) { | |
| // Allow the agent to explicitly specify a reply_to_id (e.g. for workflow_dispatch-triggered | |
| // workflows that know the target comment node ID). Apply resolveTopLevelDiscussionCommentId | |
| // to handle cases where the caller passes a reply node ID instead of a top-level one. | |
| replyToId = await resolveTopLevelDiscussionCommentId(githubClient, normalizedExplicitReplyToId); |
|
Review comments |
Centralize normalization of message.reply_to_id: coerce to String, trim whitespace, and warn+ignore when the result is empty. The normalized value (normalizedExplicitReplyToId) is computed once before the try/catch so both the main discussion path and the 404 discussion fallback path share the same validated value. Add two new tests: whitespace-only reply_to_id (warns and posts top-level) and numeric reply_to_id (coerced to string "12345"). Agent-Logs-Url: https://github.com/github/gh-aw/sessions/52b7c846-0f5e-4b30-9ce4-9358a3d5bc75 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Applied in commit d08ee35. |
add-commentonly auto-threaded discussion replies fordiscussion_comment-triggered workflows. Workflows triggered byworkflow_dispatch(e.g. dispatched specialist agents) had no way to thread their reply under a specific comment, always producing top-level comments.Changes
if (isDiscussion)block:replyToIdnow falls back tomessage.reply_to_idwhen the auto-thread path doesn't apply (not a barediscussion_commenttrigger without explicititem_number).resolveTopLevelDiscussionCommentIdis applied so nested reply node IDs are resolved to their top-level parent.404-fallback path: When an explicit
item_numberresolves to a Discussion (REST API → 404 → GraphQL retry),reply_to_idfrom the message is now honoured instead of hardcodingnull. This is the primary path forworkflow_dispatch-triggered workflows.aw_contextpropagation:buildAwContext()andresolveItemContext()now include acomment_node_idfield. Fordiscussion/discussion_commentevents, this capturespayload.comment.node_id(the GraphQL node ID of the triggering comment). Dispatched specialist workflows receive this value inaw_context.comment_node_idand can pass it directly asreply_to_idin theiradd_commentoutput to thread replies under the triggering comment.Usage
Agent output:
{ "type": "add_comment", "item_number": 240, "body": "🔄 Updated finding...", "reply_to_id": "DC_kwDOParentComment456" }reply_to_idaccepts both top-level and nested comment node IDs — nested IDs are automatically resolved to the top-level parent (GitHub Discussions supports only two nesting levels).When the trigger is
discussion_commentwithout an explicititem_number, the existing auto-thread behaviour takes precedence over anyreply_to_idin the message.End-to-end dispatch chain
discussion_comment) dispatches a specialist viadispatch_workflow—aw_context.comment_node_idcarries the GraphQL node ID of the triggering comment.workflow_dispatch, readsaw_contextfrom its inputs, and setsreply_to_idtoaw_context.comment_node_idin itsadd_commentoutput.add_commentthreads the reply under the correct discussion comment.